Draft
Conversation
TobiGr
added
enhancement
TobiGr
force-pushed
the
feat/soundcloud_replies
branch
4 times, most recently
from
75d875f to
9ae1be7
Compare
Stypox
requested changes
Dec 31, 2022
Member
Stypox
left a comment
Stypox
left a comment
There was a problem hiding this comment.
Looks mostly good to me, but it looks like at SoundCloud they overcomplicated comment replies 😅
...bi/newpipe/extractor/services/soundcloud/extractors/SoundcloudCommentsInfoItemExtractor.java
Outdated
Show resolved
Hide resolved
| } | ||
| } | ||
| replyCount = replies.size(); | ||
| if (collector.getItems().isEmpty()) { |
Member
There was a problem hiding this comment.
I guess you are collecting items here to make sure not to show that there are replies when actually all replies give an error when collected.
- Since you are already collecting replies here, can't you set the items as the
repliesPagecontent directly instead of passing thejsonagain? Or maybe make aSerializablestructure of some sort. To avoid recollecting them again later. - Or even better, I would skip this check completely and avoid collecting items beforehand. If one of the comments yields an error while being collected, then the error would be shown in the frontend and it is expected that in case of error on an item the list of replies might be empty.
When getting a page which is not the initial page there it is possible that the first comments are replies to a comment from a previous page.
TobiGr
force-pushed
the
feat/soundcloud_replies
branch
from
9ae1be7 to
e5be686
Compare
Implemented a cache. TODO: Do not store in cache when viewing replies....
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I agree to create a pull request for NewPipe as soon as possible to make it compatible with the changed API.Not needed. Is done in Show comment replies NewPipe#9410
Follow up on #936 and #941
To Do: